CloudStack Volume support with ONTAP storage#13053
CloudStack Volume support with ONTAP storage#13053rajiv-jain-netapp wants to merge 92 commits intoapache:mainfrom
Conversation
… added EOF fixes + correcting license header
Initial primary storage pool plugin skeleton
…POJOs" This reverts commit fe0f752.
…POJOs" This reverts commit 28faca1.
Feignconfiguration and volume feignClient along with desired POJOs with cstack 28
* CSTACKEX-29 Cluster, SVM and Aggr Feign Client * CSTACKEX-29 Change the endpoint method name in feign client * CSTACKEX-29 Make the alignment proper * CSTACKEX-29 Added License Info * CSTACKEX-29 Resolve Review Comments * CSTACKEX-29 Remove Component Annotation from datastoredriverclass * CSTACKEX-29 Resolve Style check issues * CSTACKEX-29 Resolve ALL Style issues * CSTACKEX-29 Resolve Precommits Issues * CSTACKEX-29 Added Method comments and change the ontap response class name --------- Co-authored-by: Gupta, Surya <Surya.Gupta@netapp.com>
* CSTACKEX-31 NAS and Job Feign Client and POJOs * CSTACKEX-31 Fixed Checks Issues * CSTACKEX-31 Resolve Review Comments * CSTACKEX-31 Resolve Review Comments * CSTACKEX-31 Resolve Review Comments * CSTACKEX-31 Added Aggr and size to volume model * CSTACKEX-31 Change the export policy endpoint path * CSTACKEX-31 Fixed check styles --------- Co-authored-by: Gupta, Surya <Surya.Gupta@netapp.com>
* CSTACKEX-30 SAN Feign Client * CSTACKEX-30 Fixed check style issues * CSTACKEX-30 Fixed review comments --------- Co-authored-by: Gupta, Surya <Surya.Gupta@netapp.com>
* CSTACKEX-7: ONTAP Primary storage pool --------- Co-authored-by: Locharla, Sandeep <Sandeep.Locharla@netapp.com>
CSTACKEX-34: Upgrade to framework classes design
* CSTACKEX-35 Create Async * CSTACKEX-35 Added Null and empty check * CSTACKEX-35 Resolved review comments * CSTACKEX-35 Removed Type Casting for logger --------- Co-authored-by: Gupta, Surya <Surya.Gupta@netapp.com>
* CSTACKEX-1: Feign changes and fixes for getting storage pool creation to work * CSTACKEX-01: Create Primary Storage pool changes with working code * CSTACKEX-01: Addressed all review comments and updated some code * CSTACKEX-01: Made some changes to fix some errors seen during testing * CSTACKEX-01: Addressed additional comments --------- Co-authored-by: Locharla, Sandeep <Sandeep.Locharla@netapp.com>
boring-cyborg
Bot
added
component:primary-storage
component:storage
component:UI
labels
There was a problem hiding this comment.
Pull request overview
This PR adds NetApp ONTAP as a managed primary storage provider, including UI wiring for pool creation and backend support for CloudStack volume workflows over NFS3 and iSCSI, plus VM/volume snapshot integration for KVM.
Changes:
- UI: adds NetApp ONTAP provider inputs (IP/username/password/SVM/protocol) and provider-specific parameter mapping.
- Backend: introduces ONTAP storage strategies (NAS/SAN), lifecycle attach/delete behavior, and Feign clients/models for snapshot + file/LUN restore workflows.
- KVM/engine: adjusts iSCSI handling and VM snapshot behavior to integrate ONTAP-specific snapshot strategy/limitations.
Reviewed changes
Copilot reviewed 42 out of 42 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/views/infra/zone/ZoneWizardLaunchZone.vue | Adds NetApp ONTAP provider handling + details mapping during zone wizard primary storage creation |
| ui/src/views/infra/zone/ZoneWizardAddResources.vue | Adds ONTAP-specific form fields and protocol choices in zone wizard |
| ui/src/views/infra/zone/StaticInputsForm.vue | Adds numeric-format validation helper used by static forms |
| ui/src/views/infra/AddPrimaryStorage.vue | Adds ONTAP provider form fields, validation rules, and API param mapping |
| ui/public/locales/en.json | Adds English i18n strings for ONTAP fields/tooltips |
| server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java | Adds ONTAP-specific error message when memory snapshots requested without a suitable strategy |
| plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java | Adds unit tests for UnifiedNASStrategy behaviors |
| plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java | Adds unit tests for StorageStrategy connect/volume/network methods |
| plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java | Extends lifecycle tests for init/attach cluster/zone flows |
| plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java | Adds driver tests for create/delete/grant/revoke access flows |
| plugins/storage/volume/ontap/src/main/resources/META-INF/cloudstack/storage-volume-ontap/spring-storage-volume-ontap-context.xml | Registers ONTAP VM snapshot strategy bean |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java | Adds ONTAP helpers for auth headers, naming, and protocol-specific volume request building |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java | Updates provider naming and adds constants for snapshot/mount options |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/CloudStackVolume.java | Extends request/response model to support snapshot workflows (flexvol UUID, destination path) |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/AccessGroup.java | Refactors access-group request to use storagePoolId instead of PrimaryDataStoreInfo |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java | Implements iSCSI LUN CRUD, igroup management, LUN mapping helpers, and snapshot revert via CLI API |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java | Implements NFS volume handling via host commands + export policy management + snapshot revert via CLI API |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java | Centralizes Feign client creation and adds snapshot job polling + snapshot feign accessors |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/SANStrategy.java | Adds SAN helper contract for ensuring LUN mappings and initiator validation |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/provider/StorageProviderFactory.java | Selects unified NAS/SAN strategy by protocol and injects components |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java | Passes pool details to agent on host connect and updates pool host refs/capacity |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java | Refactors initialization inputs, forces NFSv3 mount options, and adds attach/delete logic updates |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/VolumeConcise.java | Adds concise Volume model used by snapshot responses |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/SnapshotFileRestoreRequest.java | Adds request model for snapshot file restore API |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/OntapStorage.java | Refactors ONTAP connection model to use storageIP and removes disaggregated flag |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/LunRestoreRequest.java | Adds request model for LUN restore API |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/FlexVolSnapshot.java | Adds model for FlexVolume-level snapshots |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/FileInfo.java | Adjusts file model fields for NAS operations |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/FileClone.java | Adds request model for ONTAP file clone |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/CliSnapshotRestoreRequest.java | Adds CLI passthrough restore-file request model |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SnapshotFeignClient.java | Adds Feign client for FlexVol snapshot and restore APIs (including CLI passthrough) |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java | Adds Feign method for LUN restore |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/NASFeignClient.java | Updates file GET to include metadata and adds file clone endpoint |
| plugins/storage/volume/ontap/pom.xml | Adds dependencies and surefire argLine for tests |
| plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java | Makes iSCSI login/create idempotent, rescans sessions, avoids logging out when other LUNs exist, adds flush/sync |
| plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCreateDiskOnlyVMSnapshotCommandWrapper.java | Improves error handling and consolidates cleanup logic for snapshot deltas |
| engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java | Updates managed template copy flow to use iSCSI/LUN info available post-grantAccess for ONTAP |
| engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java | Excludes ONTAP managed pools from the generic KVM file-based VM snapshot strategy and improves failure messages |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #13053 +/- ##
============================================
+ Coverage 17.95% 18.02% +0.07%
- Complexity 16503 16674 +171
============================================
Files 6019 6043 +24
Lines 540748 542990 +2242
Branches 66255 66612 +357
============================================
+ Hits 97084 97877 +793
- Misses 432723 434115 +1392
- Partials 10941 10998 +57
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@DaanHoogland @winterhazel , How can we re-trigger the build? |
@rajiv-jain-netapp you can ping @blueorangutan and use the package command, like this: @blueorangutan package |
|
@winterhazel a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17610 |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian Build Failed (tid-15969) |
|
[SF] Trillian test result (tid-15974)
|
|
@weizhouapache @winterhazel , at your discretion (ready for merge for my part) |
winterhazel
left a comment
winterhazel
left a comment
There was a problem hiding this comment.
Currently reviewing. This will take some time, so I am submitting the reviews partially.
|
|
||
| private static final List<Storage.StoragePoolType> supportedStoragePoolTypes = List.of(Storage.StoragePoolType.Filesystem, Storage.StoragePoolType.NetworkFilesystem, Storage.StoragePoolType.SharedMountPoint); | ||
|
|
||
| private static final String ONTAP_PROVIDER_NAME = "NetApp ONTAP"; |
There was a problem hiding this comment.
This constant is declared in 3 places. Perhaps we could create a file in a single common dependency (maybe cloud-api) to declare the plugin names?
| grantAccess(volumeInfo, destHost, primaryDataStore); | ||
| volumeInfo = volFactory.getVolume(volumeInfo.getId(), primaryDataStore); | ||
| // For Netapp ONTAP iscsiName or Lun path is available only after grantAccess | ||
| String managedStoreTarget = volumeInfo.get_iScsiName() != null ? volumeInfo.get_iScsiName() : volumeInfo.getUuid(); |
There was a problem hiding this comment.
| String managedStoreTarget = volumeInfo.get_iScsiName() != null ? volumeInfo.get_iScsiName() : volumeInfo.getUuid(); | |
| String managedStoreTarget = ObjectUtils.defaultIfNull(volumeInfo.get_iScsiName(), volumeInfo.getUuid()); |
There was a problem hiding this comment.
This is replacing the ternary on line 1345 with a call to ObjectUtils.defaultIfNull. They essentially do the same thing, but we tend to use the latter for legibility and avoiding two calls to the nullable value (volumeInfo.get_iScsiName())
| primaryDataStore.setDetails(details); | ||
| // Update destTemplateInfo with the iSCSI path from volumeInfo | ||
| if (destTemplateInfo instanceof TemplateObject) { | ||
| ((TemplateObject)destTemplateInfo).setInstallPath(volumeInfo.getPath()); |
There was a problem hiding this comment.
This setInstallPath here may affect other primary storage solutions. Is it possible to place it inside a method of the ONTAP plugin's driver, or only execute it if ONTAP is being used?
| @JsonProperty("uuid") | ||
| private String uuid; | ||
| @JsonProperty("name") | ||
| private String name; | ||
| public String getUuid() { | ||
| return uuid; | ||
| } | ||
| public void setUuid(String uuid) { | ||
| this.uuid = uuid; | ||
| } | ||
| public String getName() { | ||
| return name; | ||
| } | ||
| public void setName(String name) { this.name = name; } |
There was a problem hiding this comment.
| @JsonProperty("uuid") | |
| private String uuid; | |
| @JsonProperty("name") | |
| private String name; | |
| public String getUuid() { | |
| return uuid; | |
| } | |
| public void setUuid(String uuid) { | |
| this.uuid = uuid; | |
| } | |
| public String getName() { | |
| return name; | |
| } | |
| public void setName(String name) { this.name = name; } | |
| @JsonProperty("uuid") | |
| private String uuid; | |
| @JsonProperty("name") | |
| private String name; | |
| public String getUuid() { | |
| return uuid; | |
| } | |
| public void setUuid(String uuid) { | |
| this.uuid = uuid; | |
| } | |
| public String getName() { | |
| return name; | |
| } | |
| public void setName(String name) { | |
| this.name = name; | |
| } |
There was a problem hiding this comment.
It is a formatting suggestion. Leaving a blank line between method/attribute declarations, and not declaring methods in a single line.
| } | ||
| public String getFlexVolumeUuid() { | ||
| return flexVolumeUuid; | ||
| } |
There was a problem hiding this comment.
@winterhazel what is the recommended change/comment?
There was a problem hiding this comment.
Having a blank line before/after the declaration of method getFlexVolumeUuid.
| public String getDestinationPath() { return this.destinationPath; } | ||
| public void setDestinationPath(String destinationPath) { this.destinationPath = destinationPath; } |
There was a problem hiding this comment.
In our previous PR, we had been told to maintain the single-line method implementation as above.
There was a problem hiding this comment.
I think there are no rules for this specific case in our coding conventions, but we tend to declare all methods in multiple lines instead of a single one.
There was a problem hiding this comment.
I never heard such a recommendation before @rajiv-jain-netapp . It is not usual.
There was a problem hiding this comment.
Sure I will update them
| // Inject mocks into the inherited fields via reflection | ||
| // DefaultVMSnapshotStrategy fields | ||
| setField(strategy, DefaultVMSnapshotStrategy.class, "vmSnapshotHelper", vmSnapshotHelper); | ||
| setField(strategy, DefaultVMSnapshotStrategy.class, "guestOSDao", guestOSDao); | ||
| setField(strategy, DefaultVMSnapshotStrategy.class, "userVmDao", userVmDao); | ||
| setField(strategy, DefaultVMSnapshotStrategy.class, "vmSnapshotDao", vmSnapshotDao); | ||
| setField(strategy, DefaultVMSnapshotStrategy.class, "agentMgr", agentMgr); | ||
| setField(strategy, DefaultVMSnapshotStrategy.class, "volumeDao", volumeDao); | ||
|
|
||
| // StorageVMSnapshotStrategy fields | ||
| setField(strategy, StorageVMSnapshotStrategy.class, "storagePool", storagePool); | ||
| setField(strategy, StorageVMSnapshotStrategy.class, "vmSnapshotDetailsDao", vmSnapshotDetailsDao); | ||
| setField(strategy, StorageVMSnapshotStrategy.class, "volumeDataFactory", volumeDataFactory); | ||
|
|
||
| // OntapVMSnapshotStrategy fields | ||
| setField(strategy, OntapVMSnapshotStrategy.class, "storagePoolDetailsDao", storagePoolDetailsDao); | ||
| setField(strategy, OntapVMSnapshotStrategy.class, "volumeDetailsDao", volumeDetailsDao); |
There was a problem hiding this comment.
Please use @InjectMocks instead of reflection
| } catch (Exception e) { | ||
| logger.error("Unexpected exception while creating disk-only VM snapshot for VM [{}]. Deleting leftover deltas.", vmName, e); | ||
| cleanupLeftoverDeltas(volumeObjectTos, mapVolumeToSnapshotSizeAndNewVolumePath, storagePoolMgr); | ||
| return new CreateDiskOnlyVmSnapshotAnswer(cmd, false, | ||
| String.format("Creation of disk-only VM snapshot for VM [%s] failed due to %s.", vmName, e.getMessage()), null); |
There was a problem hiding this comment.
Can this be unified with the catch block above?
There was a problem hiding this comment.
@winterhazel, I added this logic separately after encountering failures in negative scenarios where the UI did not surface any error details. With this change, users are now notified of the failure reason when the exception falls outside the scope of the existing catch block. This is expected to improve overall user experience. Please let me know if you still feel this should be removed.
| @@ -158,8 +183,23 @@ public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map<S | |||
| return true; | |||
| } | |||
There was a problem hiding this comment.
This result check block could be extracted into a single method to reduce indentation and make unit testing easier
There was a problem hiding this comment.
@winterhazel I introduced this nested logic because NetApp storage exposes IQNs at the SVM level rather than at the volume level. As a result, all volumes within the same SVM share the same IQN when creating physical disks. This check ensures that if a connection to the SVM‑level IQN already exists, the failure can be safely ignored; otherwise, an exception should be raised.
| @@ -123,21 +130,39 @@ public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map<S | |||
| } | |||
| } | |||
There was a problem hiding this comment.
This result check block could be extracted into a single method to reduce indentation and make unit testing easier
There was a problem hiding this comment.
@winterhazel can you please elaborate on this comment?
| */ | ||
| private boolean hasOtherActiveLuns(String host, int port, String iqn, String lun) { | ||
| String prefix = "ip-" + host + ":" + port + "-iscsi-" + iqn + "-lun-"; | ||
| java.io.File byPathDir = new java.io.File("/dev/disk/by-path"); |
There was a problem hiding this comment.
Please import the File class and use only the classname
| logger.info("Other LUNs still active after removing /" + iqn + "/" + lun + " — session kept alive."); | ||
| return true; |
There was a problem hiding this comment.
As the disconnect operation failed, I wonder if it makes more sense to throw an exception here instead
| logger.error("Failed to add host by HostListener as host was not found with id : {}", host.getId()); | ||
| return false; | ||
| } | ||
| // TODO add storage pool get validation |
There was a problem hiding this comment.
Is this going to be addressed in this PR?
| return false; | ||
| } | ||
|
|
||
| // TODO add host type check also since we support only KVM for now, host.getHypervisorType().equals(HypervisorType.KVM) |
There was a problem hiding this comment.
Is this going to be addressed in this PR?
| throw new CloudRuntimeException("Unsupported configuration: Disaggregated ONTAP is not supported."); | ||
| UnifiedNASStrategy unifiedNASStrategy = new UnifiedNASStrategy(ontapStorage); | ||
| ComponentContext.inject(unifiedNASStrategy); | ||
| unifiedNASStrategy.setOntapStorage(ontapStorage); |
There was a problem hiding this comment.
Is this set needed? ontapStorage is already passed as a parameter to unifiedNASStrategy
| throw new CloudRuntimeException("Unsupported configuration: Disaggregated ONTAP is not supported."); | ||
| UnifiedSANStrategy unifiedSANStrategy = new UnifiedSANStrategy(ontapStorage); | ||
| ComponentContext.inject(unifiedSANStrategy); | ||
| unifiedSANStrategy.setOntapStorage(ontapStorage); |
There was a problem hiding this comment.
Is this set needed? ontapStorage is already passed as a parameter to unifiedSANStrategy
rajiv-jain-netapp
force-pushed
the
sync/apache-main-apr-2026
branch
from
10cb764 to
c86d377
Compare
| port = OntapStorageConstants.NFS3_PORT; | ||
| logger.info("Setting NFS path for storage pool: " + path + ", port: " + port); | ||
| // Force NFSv3 for ONTAP managed storage to avoid NFSv4 ID mapping issues | ||
| details.put(OntapStorageConstants.NFS_MOUNT_OPTIONS, OntapStorageConstants.NFS3_MOUNT_OPTIONS_VER_3); |
There was a problem hiding this comment.
We can use ApiConstants.NFS_MOUNT_OPTIONS instead of OntapStorageConstants.NFS_MOUNT_OPTIONS
| if (podId == null) { | ||
| if (zoneId != null) { | ||
| logger.info("Both Pod Id and Cluster Id are null, Primary storage pool will be associated with a Zone"); | ||
| } else { | ||
| throw new CloudRuntimeException("Pod Id, Cluster Id and Zone Id are all null, cannot create primary storage"); | ||
| } | ||
| } |
There was a problem hiding this comment.
These two ifs can be reduced to a single if (podId == null && zoneId != null) {
|
|
||
| // Basic parameter validation | ||
| if (StringUtils.isBlank(storagePoolName)) { | ||
| if (storagePoolName == null || storagePoolName.isEmpty()) { |
There was a problem hiding this comment.
Why was StringUtils.isBlank replaced here?
| } | ||
| if (StringUtils.isBlank(providerName)) { | ||
|
|
||
| if (providerName == null || providerName.isEmpty()) { |
There was a problem hiding this comment.
Why was StringUtils.isBlank replaced here?
| throw new CloudRuntimeException("Unexpected ONTAP detail key in URL: " + key); | ||
| } | ||
| if (StringUtils.isBlank(val)) { | ||
| if (val == null || val.isEmpty()) { |
There was a problem hiding this comment.
Why was StringUtils.isBlank replaced here?
| String accessGroupName = OntapStorageUtils.getIgroupName(svmName, host.getName()); | ||
|
|
||
| // Retrieve LUN name from volume details; if missing, volume may not have been fully created | ||
| String lunName = volumeDetailsDao.findDetail(volumeVO.getId(), OntapStorageConstants.LUN_DOT_NAME) != null ? |
There was a problem hiding this comment.
Extract volumeDetailsDao.findDetail(volumeVO.getId(), OntapStorageConstants.LUN_DOT_NAME) into a variable to avoid calling it twice
| // For iSCSI, retrieve LUN UUID for restore operations | ||
| String lunUuid = null; | ||
| if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol)) { | ||
| lunUuid = volumeDetailsDao.findDetail(volumeVO.getId(), OntapStorageConstants.LUN_DOT_UUID) != null |
There was a problem hiding this comment.
Extract volumeDetailsDao.findDetail(volumeVO.getId(), OntapStorageConstants.LUN_DOT_UUID) into a variable to avoid callign it twice
| String jobUUID = jobResponse.getJob().getUuid(); | ||
|
|
||
| Boolean jobSucceeded = jobPollForSuccess(jobUUID); | ||
| Boolean jobSucceeded = jobPollForSuccess(jobUUID,10, 1); |
There was a problem hiding this comment.
| Boolean jobSucceeded = jobPollForSuccess(jobUUID,10, 1); | |
| Boolean jobSucceeded = jobPollForSuccess(jobUUID, 10, 1); |
| logger.info("Deleting ONTAP volume by name: " + volume.getName() + " and uuid: " + volume.getUuid()); | ||
| String authHeader = OntapStorageUtils.generateAuthHeader(storage.getUsername(), storage.getPassword()); | ||
| try { | ||
| // TODO: Implement lun and file deletion, if any, before deleting the volume |
There was a problem hiding this comment.
Is this going to be addressed in this PR, or separately?
| // TODO: Implement lun and file deletion, if any, before deleting the volume | ||
| JobResponse jobResponse = volumeFeignClient.deleteVolume(authHeader, volume.getUuid()); | ||
| Boolean jobSucceeded = jobPollForSuccess(jobResponse.getJob().getUuid()); | ||
| Boolean jobSucceeded = jobPollForSuccess(jobResponse.getJob().getUuid(),10, 1); |
There was a problem hiding this comment.
| Boolean jobSucceeded = jobPollForSuccess(jobResponse.getJob().getUuid(),10, 1); | |
| Boolean jobSucceeded = jobPollForSuccess(jobResponse.getJob().getUuid(), 10, 1); |
| logger.info("createCloudStackVolume: Create cloudstack volume " + cloudstackVolume); | ||
| try { | ||
| // Step 1: set cloudstack volume metadata | ||
| String volumeUuid = updateCloudStackVolumeMetadata(cloudstackVolume.getDatastoreId(), cloudstackVolume.getVolumeInfo()); |
| try { | ||
| VolumeObject volumeObject = (VolumeObject) volumeInfo; | ||
| long volumeId = volumeObject.getId(); | ||
| logger.info("updateCloudStackVolumeMetadata: VolumeInfo ID from VolumeObject: {}", volumeId); | ||
| VolumeVO volume = volumeDao.findById(volumeId); | ||
| if (volume == null) { | ||
| throw new CloudRuntimeException("Volume not found with id: " + volumeId); | ||
| } | ||
| String volumeUuid = volumeInfo.getUuid(); | ||
| volume.setPoolType(Storage.StoragePoolType.NetworkFilesystem); | ||
| volume.setPoolId(Long.parseLong(dataStoreId)); | ||
| volume.setPath(volumeUuid); // Filename for qcow2 file | ||
| volumeDao.update(volume.getId(), volume); | ||
| logger.info("Updated volume path to {} for volume ID {}", volumeUuid, volumeId); | ||
| return volumeUuid; | ||
| }catch (Exception e){ | ||
| logger.error("updateCloudStackVolumeMetadata: Exception while updating volumeInfo: {} in volume: {}", dataStoreId, volumeInfo.getUuid(), e); | ||
| throw new CloudRuntimeException("Exception while updating volumeInfo: " + e.getMessage()); | ||
| } |
There was a problem hiding this comment.
Adjust the indentation of this try block. All lines are missing a space
| CloudStackVolume cloudStackVolume = null; | ||
| FileInfo fileInfo = getFile(cloudStackVolumeMap.get(OntapStorageConstants.VOLUME_UUID),cloudStackVolumeMap.get(OntapStorageConstants.FILE_PATH)); | ||
|
|
||
| if(fileInfo != null){ |
There was a problem hiding this comment.
| if(fileInfo != null){ | |
| if (fileInfo != null) { |
| @@ -180,11 +212,11 @@ public void disableLogicalAccess(Map<String, String> values) { | |||
|
|
|||
| @Override | |||
| public Map<String, String> getLogicalAccess(Map<String, String> values) { | |||
There was a problem hiding this comment.
This method does not seem to be used outside the tests. Is this expected?
| } | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
Methods from line 301-367 are not used. Is this expected?
| return name.matches(OntapStorageConstants.ONTAP_NAME_REGEX); | ||
| } | ||
|
|
||
| public static String getOSTypeFromHypervisor(String hypervisorType){ |
There was a problem hiding this comment.
| public static String getOSTypeFromHypervisor(String hypervisorType){ | |
| public static String getOSTypeFromHypervisor(String hypervisorType) { |
Description
Co-authored-by: Sandeep Locharla sandeep.locharla@netapp.com
Co-authored-by: Piyush Srivastava piyush5@netapp.com
Co-authored-by: Surya Gupta suryag@netapp.com
This PR...
1. memory snapshot
2. user input for quicing option during snapshot creation
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
Testing Done:
How did you try to break this feature and the system with this change?